-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Clean up log4j-spring-cloud-config-client dependencies
#2166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
463e791 to
fc01336
Compare
f3492c8 to
bb29376
Compare
|
It seems to be OK, however from my personal point of view I would say that Spring related properties should not be in |
vy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recently put significant effort to get log4j-parent rid off Spring-related dependencies. @raffig agrees with the earlier design too, see his comment. Could you please update the PR such that log4j-parent is untouched and log4j-spring-cloud-config-client changes only contain the fix to #2157, please?
|
@vy, I moved the Spring dependencies back to Since these dependency-management-polluting artifacts are out of the picture, I don't see a difference between |
|
I would say that dependencies specific to |
|
To be clear, the parent module @vy, why should |
We minimize the dependencies of `log4j-spring-cloud-config-client` to those really required: `spring-context` and `spring-cloud-context`. The dependency on `spring-boot-autoconfigure` is optional (its just used in annotations) and the artifact can be also used without Spring Boot. Fixes apache#2157.
e157719 to
8bbc085
Compare
|
@ppkarwasz thank you so much for your effort and understanding in this PR. You also have done a great job by introducing our agreement here as a general policy to the Log4j team. All in all, thank you very much! 🙇 |
We minimize the dependencies of
log4j-spring-cloud-config-clientto those really required:spring-contextandspring-cloud-context.The dependency on
spring-boot-autoconfigureis optional (its just used in annotations) and the artifact can be also used without Spring Boot.Fixes #2157.